Repository navigation
feat: add policy layout components and enhance policy details page - #273
Conversation
- Introduced new layout components for policy overview and individual policy pages, improving structure and navigation. - Updated PolicyDetails page to include a PolicyPageEditor for editing policy content. - Removed deprecated PolicyEditor component to streamline the codebase. - Implemented responsive design with Tailwind CSS for better user experience.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe changes update the policy editing functionality and layout structure across multiple files. A key function export has been renamed from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PolicyPageEditor
participant SaveHandler
User->>PolicyPageEditor: Edit and update policy content
PolicyPageEditor->>SaveHandler: Invoke handleSavePolicy()
SaveHandler-->>PolicyPageEditor: Return save status
PolicyPageEditor-->>User: Display update result
sequenceDiagram
participant Browser
participant Layout
participant i18nService
participant SecondaryMenu
Browser->>Layout: Request policy page
Layout->>i18nService: Fetch internationalization strings
i18nService-->>Layout: Return localized content
Layout->>SecondaryMenu: Render navigation items
Layout-->>Browser: Display layout with children content
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/layout.tsx (1)
11-11: Unused variable in the componentThe
policyIdis extracted from params but not used in the component rendering. Consider removing it if not needed.- const { orgId, policyId } = await params; + const { orgId } = await params;apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx (1)
24-29: Consider improving error handlingThe current error handling logs the error but then rethrows it, which could lead to unhandled promise rejections if the caller doesn't catch errors. Consider adding a more user-friendly error handling approach.
try { await updatePolicy({ policyId, content: policyContent }); } catch (error) { console.error("Error saving policy:", error); - throw error; + // Consider adding toast notification or similar user feedback + // toast.error("Failed to save policy. Please try again."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/page.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/layout.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/layout.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/page.tsx
🧰 Additional context used
🧬 Code Definitions (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/actions/update-policy.ts (1)
updatePolicy(31-132)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/layout.tsx (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/layout.tsx (1)
Layout(9-31)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/(overview)/layout.tsx (1)
Layout(9-31)
🔇 Additional comments (7)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/layout.tsx (2)
1-31: Well-structured layout component with consistent designThis new layout component follows the established pattern seen in other policy layout files, providing a consistent navigation structure with internationalization support. The secondary menu implementation ensures users can easily navigate between policy views.
4-7:Details
❓ Verification inconclusive
Consider revising the params type definition
The
paramsproperty is defined as a Promise, but it's worth checking if this is necessary. In Next.js 13+, route params are typically directly available as objects rather than Promises.
🏁 Script executed:
#!/bin/bash # Check other layout components in the project to verify params pattern rg -A 1 -B 1 "interface.*Props" --glob "**/*layout.tsx" | grep -A 3 "params"Length of output: 76
Action Required: Revisit the 'params' Type Definition
The initial automated check did not reveal any similar usage of a Promise-wrapped params in other layout components. In Next.js 13, route parameters are usually directly available as objects rather than Promises. Please verify manually whether the Promise wrapper is intentional in this context or if the type should be updated to a plain object:
interface LayoutProps { children: React.ReactNode; params: { policyId: string; orgId: string }; }Ensure to check any related logic that might expect the async resolution. If no asynchronous behavior is needed, consider revising accordingly.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/all/layout.tsx (1)
9-31: Layout implementation looks goodThe layout provides consistent navigation across policy views and properly handles internationalization. The component correctly extracts only the
orgIdfrom params.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/page.tsx (2)
9-12: Appropriate imports for the updated componentThe added imports for
JSONContentandPolicyPageEditorsupport the enhanced functionality of the policy page.
13-38: Good integration of policy view and edit componentsThe page now properly integrates both the policy overview and editor components, providing a complete policy management interface. The breadcrumb navigation is also well-implemented.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx (2)
12-12: Component renaming improves clarityRenaming from
PolicyDetailstoPolicyPageEditorbetter reflects the component's purpose and aligns with its usage in the page component.
33-33: Styling enhancement with border and paddingThe styling change adds visual structure around the editor, making it clearer where the editable area begins and ends.
| interface LayoutProps { | ||
| children: React.ReactNode; | ||
| params: Promise<{ policyId: string; orgId: string }>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Revise the params interface for the all policies layout
The LayoutProps interface includes policyId, but this isn't applicable for the "all policies" view which doesn't have a specific policy ID in its route parameters.
interface LayoutProps {
children: React.ReactNode;
- params: Promise<{ policyId: string; orgId: string }>;
+ params: Promise<{ orgId: string }>;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface LayoutProps { | |
| children: React.ReactNode; | |
| params: Promise<{ policyId: string; orgId: string }>; | |
| } | |
| interface LayoutProps { | |
| children: React.ReactNode; | |
| params: Promise<{ orgId: string }>; | |
| } |
| <PolicyPageEditor | ||
| policyId={policyId} | ||
| policyContent={policy?.content as JSONContent[]} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check for policy content
The code casts policy?.content directly to JSONContent[] without verification. This might cause runtime errors if the content is null or undefined.
<PolicyPageEditor
policyId={policyId}
- policyContent={policy?.content as JSONContent[]}
+ policyContent={policy?.content ? (policy.content as JSONContent[]) : []}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <PolicyPageEditor | |
| policyId={policyId} | |
| policyContent={policy?.content as JSONContent[]} | |
| /> | |
| <PolicyPageEditor | |
| policyId={policyId} | |
| policyContent={policy?.content ? (policy.content as JSONContent[]) : []} | |
| /> |
…, GH-79, GH-78, GH-53, GH-54 (#3576) * fix(mcp-server): bind serve to localhost and reject foreign origins GH-273: the streamable HTTP server bound 0.0.0.0 with Access-Control-Allow-Origin: * and no request authentication, so any website the operator visited could drive the local MCP server with the CLI-configured API key, and the LAN could reach it too. - bind to 127.0.0.1 by default (new --host flag to opt out) - drop the wildcard CORS middleware (MCP HTTP clients are not browsers) - reject POST /mcp with a non-localhost Origin header (browsers always send one; header-less local MCP clients keep working with --apikey) * fix(portal): scope policy signature and pdf endpoints to caller's org GH-93 / GH-79 / GH-245: portal policy endpoints looked policies and policy versions up by bare id with no organization scoping, letting any authenticated portal user sign policies in other tenants or fetch a presigned URL for another org's policy PDF. - mark-policy-completed: resolve the member within the policy's organization (also fixes the arbitrary-membership pick for multi-org users) - accept-policies: skip policies outside the member's organization - policy-pdf-url: scope the versionId lookup to the policy whose org membership was already validated * fix(security): mask all text and media in sentry session replay GH-101: session replay ran in production with maskAllText: false and blockAllMedia: false while the sentry-mask escape hatch was used in zero of ~1470 components, recording customer compliance data verbatim. Default to masking everything; data-sentry-unmask is now the opt-in for elements provably safe to record. * fix(email): remove hardcoded fallback secret for unsubscribe tokens GH-100: the unsubscribe-token HMAC key silently fell back to the public literal 'fallback-secret' when neither UNSUBSCRIBE_SECRET nor AUTH_SECRET was set, and apps/api (which never sets either) verifies the unauthenticated POST /v1/email/unsubscribe endpoint with it, making every token forgeable from source. Resolve the secret lazily and fail closed: generating a token without a configured secret now throws. Documented UNSUBSCRIBE_SECRET in apps/api/.env.example. * fix(api): validate organizationId before building upstash vector filters GH-103: findSimilarContent and findSimilarContentBatch interpolated organizationId into the Upstash Vector metadata filter with no escaping, so a value containing a quote could append arbitrary filter clauses (e.g. OR organizationId GLOB "*") and dump every tenant's RAG chunks. Allowlist the prefixed-CUID shape and fail closed before the filter string is built, in both query paths and the sync readiness check. Adds regression tests for the literal exploit payload. * fix(api): honor securityQuestionnaireEnabled on token questionnaire upload GH-78: POST /v1/questionnaire/parse/upload/token validated the trust access token and ran RAG auto-answering without checking Trust.securityQuestionnaireEnabled, so an org that disabled the AI questionnaire was still fully served by any token holder. Check the flag after token validation and return 403 when disabled; defaults to enabled when no Trust row exists, matching the public overview helper's semantics. * fix(api): scope organization logo keys to the owning organization GH-53: UpdateOrganizationDto.logo accepted any string, it was stored verbatim, and GET /v1/organization presigned it as a raw S3 key against the shared org-assets bucket - a cross-tenant read of any known key. Reject logo keys not prefixed with the organization's own id on update, refuse to presign out-of-org keys on read, and skip presigning out-of-org logos in the app layout. * fix(api): reject cross-organization fileKeys in evidence form submissions GH-54: evidence form file fields accepted any non-empty fileKey, stored it verbatim, and the submissions read path presigned it against the shared attachments bucket - a cross-tenant read given a leaked key. Validate every submitted fileKey against the caller's organization prefix on create, and skip presigning out-of-org keys when refreshing download URLs on read (defense in depth for legacy rows). * fix(app): verify run ownership in task status route via org tags GH-257: GET /api/tasks/[taskId]/status returned any Trigger.dev run's output to any authenticated user with no ownership check, exposing other tenants' AI-generated policy, questionnaire, and vendor content to whoever learned a run id. Tag every user-triggerable tasks.trigger call with the owning organization id and require a matching tag before returning run data; mismatches get the same 404 as a missing run. research-vendor runs are tagged when an active org exists (its output is shared, not tenant data). Adds route tests for the cross-tenant and untagged cases. * fix: refine access checks and legacy MCP maintenance * fix(security): address review feedback on csv export, batch fix tags and run status rbac - exportCsv skips presigning fileKeys outside the organization prefix - derive the batch fix run tag from the created batch, not caller input - require task:read on the task run status route --------- Co-authored-by: tofikwest <tofik@trycomp.ai>
Summary by CodeRabbit
New Features
Style
Removed Features